-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump glob to 10.3.12 #16450
Bump glob to 10.3.12 #16450
Conversation
if (process.env.BABEL_8_BREAKING) { | ||
// glob 9+ no longer sorts the result, here we maintain the glob 7 behaviour | ||
// https://github.com/isaacs/node-glob/blob/c3cd57ae128faa0e9190492acc743bb779ac4054/common.js#L151 | ||
// eslint-disable-next-line no-var | ||
var files = globSync(input, { dotRelative: true }).sort( | ||
function alphasort(a, b) { | ||
return a.localeCompare(b, "en"); | ||
}, | ||
); | ||
} else { | ||
files = globSync(input); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (process.env.BABEL_8_BREAKING) { | |
// glob 9+ no longer sorts the result, here we maintain the glob 7 behaviour | |
// https://github.com/isaacs/node-glob/blob/c3cd57ae128faa0e9190492acc743bb779ac4054/common.js#L151 | |
// eslint-disable-next-line no-var | |
var files = globSync(input, { dotRelative: true }).sort( | |
function alphasort(a, b) { | |
return a.localeCompare(b, "en"); | |
}, | |
); | |
} else { | |
files = globSync(input); | |
} | |
let files = globSync(input, { dotRelative: true }); | |
if (process.env.BABEL_8_BREAKING) { | |
// glob 9+ no longer sorts the result, here we maintain the glob 7 behaviour | |
// https://github.com/isaacs/node-glob/blob/c3cd57ae128faa0e9190492acc743bb779ac4054/common.js#L151 | |
files.sort( | |
function alphasort(a, b) { | |
return a.localeCompare(b, "en"); | |
}, | |
); | |
} |
Curious if this works (to avoid var
in the Babel 8 code, minimizing the manual changes when we'll codemod away process.env.BABEL_8_BREAKING
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does work technically since glob 7 does not validate its options, as dotRelative
was added in glob 9.1.
64ec757
to
e851f26
Compare
e851f26
to
52efd13
Compare
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/56791 |
packages/babel-cli/package.json
Outdated
@@ -28,7 +28,7 @@ | |||
"commander": "^4.0.1", | |||
"convert-source-map": "^2.0.0", | |||
"fs-readdir-recursive": "^1.1.0", | |||
"glob": "^7.2.0", | |||
"glob": "condition:BABEL_8_BREAKING ? ^10.3.12 : ^7.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"glob": "condition:BABEL_8_BREAKING ? ^10.3.12 : ^7.2.0", | |
"glob": "condition:BABEL_8_BREAKING ? ^10.3.12 : ^7.2.0 (esm:sync)", |
this should fix CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I just figured out the same change from the conditionParser.ts.
let files = process.env.BABEL_8_BREAKING | ||
? // glob 9+ no longer sorts the result, here we maintain the glob 7 behaviour | ||
// https://github.com/isaacs/node-glob/blob/c3cd57ae128faa0e9190492acc743bb779ac4054/common.js#L151 | ||
globSync(input, { dotRelative: true }).sort(function alphasort(a, b) { | ||
return a.localeCompare(b, "en"); | ||
}) | ||
: globSync(input); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using the plugin, would this work?
import * as glob from "glob";
let files = process.env.BABEL_8_BREAKING | |
? // glob 9+ no longer sorts the result, here we maintain the glob 7 behaviour | |
// https://github.com/isaacs/node-glob/blob/c3cd57ae128faa0e9190492acc743bb779ac4054/common.js#L151 | |
globSync(input, { dotRelative: true }).sort(function alphasort(a, b) { | |
return a.localeCompare(b, "en"); | |
}) | |
: globSync(input); | |
let files = process.env.BABEL_8_BREAKING | |
? // glob 9+ no longer sorts the result, here we maintain the glob 7 behaviour | |
// https://github.com/isaacs/node-glob/blob/c3cd57ae128faa0e9190492acc743bb779ac4054/common.js#L151 | |
(USE_ESM ? glob.default : glob).sync(input, { dotRelative: true }).sort(function alphasort(a, b) { | |
return a.localeCompare(b, "en"); | |
}) | |
: glob.sync(input); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The USE_ESM branch should be moved to the Babel 7, otherwise it looks good to me, I will give it a try.
New options are added to maintain the glob 7 behaviour. The import is changed so that it works for both glob 7 + CJS and glob 10 + ESM scenario. There should be no behaviour changes for babel-cli users.
We can't ship this PR for Babel 7 as glob 9 dropped Node < 14 support.